FEAT: AWS secret extension #3340

chenqi0805 · 2023-09-14T18:17:18Z

Description

This PR adds support for AWS secret extension plugin:

pipeline_extensions:
  aws:
    secrets:
      secret1:
        name: my-es-secret
        region: us-east-1
      secret2:
        name: my-es-secret-max-retries
        region: us-east-1
log-pipeline:
  workers: 1
  source:
    http:
      path: "/log/ingest"
  buffer:
    bounded_blocking:
      batch_size: ${{aws_secrets:secret1:batch_size}} # max number of records the buffer will drain for each read
  sink:
    - opensearch:
        hosts: [ "..." ]
        index: test-es7-compression
        username: ${{aws_secrets:secret1:username}}
        password: ${{aws_secrets:secret1:password}}
        flush_timeout: -1
        max_retries: ${{aws_secrets:secret2}}

The scope of settings that support secret reference is limited to plugins (source/buffer/sink/processor) in pipeline.YAML. The reference pattern is explained as follows:

${{aws_secrets:secret1:username}} means the secret value under secret1 will be parsed as json and the setting value is swapped with the one according to key username in the secret json
${{aws_secrets:secret2}} means the secret value under secret2 is parsed as a literal string and the setting value is swapped with the literal string value

Issues Resolved

Resolves #2780

Check List

New functionality includes testing.
New functionality has a documentation issue. Please link to it in this PR.
- New functionality has javadoc added
Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: George Chen <[email protected]>

…nsion Signed-off-by: George Chen <[email protected]>

Signed-off-by: George Chen <[email protected]>

…nsion Signed-off-by: George Chen <[email protected]>

Signed-off-by: George Chen <[email protected]>

github-actions · 2023-09-14T18:22:10Z

Unit Test Results

  1 448 files +  16   1 448 suites +16 46m 7s ⏱️ + 6m 17s
  6 034 tests +  67   6 033 ✔️ +  67 1 💤 ±0 0 ❌ ±0
11 798 runs +160 11 796 ✔️ +160 2 💤 ±0 0 ❌ ±0

Results for commit 73a265d. ± Comparison against base commit b2b3249.

♻️ This comment has been updated with latest results.

Signed-off-by: George Chen <[email protected]>

dlvenable · 2023-09-15T16:53:37Z

...r-api/src/main/java/org/opensearch/dataprepper/model/plugin/PluginConfigValueTranslator.java

+package org.opensearch.dataprepper.model.plugin;
+
+public interface PluginConfigValueTranslator {
+    String translate(final String value);


This should return a generic type as it can vary.

dlvenable · 2023-09-15T16:56:03Z

...plugins/aws-plugin-api/src/main/java/org/opensearch/dataprepper/aws/api/SecretsSupplier.java

@@ -0,0 +1,7 @@
+package org.opensearch.dataprepper.aws.api;
+
+public interface SecretsSupplier {


We don't want to expose this in the aws-plugin-api. It could couple plugins with AWS SecretsManager which is not desirable.

dlvenable · 2023-09-15T16:57:42Z

data-prepper-core/build.gradle

@@ -16,6 +16,7 @@ dependencies {
    implementation project(':data-prepper-expression')
    implementation project(':data-prepper-plugins:blocking-buffer')
    implementation project(':data-prepper-plugins:common')
+    implementation project(':data-prepper-plugins:aws-plugin')


Data Prepper core should not depend on aws-plugin. The extensions framework is what will let this load dynamically.

Thanks for the catch! It is unnecessary

dlvenable · 2023-09-15T16:59:19Z

data-prepper-plugins/aws-plugin/build.gradle

@@ -16,7 +20,7 @@ jacocoTestCoverageVerification {
    violationRules {
        rule {
            limit {
-                minimum = 1.0
+                minimum = 0.98


What can we do to this back to 100%?

There is one JsonProcessingException unreachable:

@Override public Object retrieveValue(String secretId) { if (!secretIdToValue.containsKey(secretId)) { throw new IllegalArgumentException(String.format("Unable to find secretId: %s", secretId)); } try { final Object secretValue = secretIdToValue.get(secretId); return secretValue instanceof Map ? OBJECT_MAPPER.writeValueAsString(secretIdToValue.get(secretId)) : secretValue; } catch (JsonProcessingException e) { throw new IllegalArgumentException(String.format("Unable to read the value under secretId: %s as string", secretId)); } }

This is about writing value back into a string when ${{aws_secrets:secretId}} is used but the actual secret Value is a valid json, which means we interpret it as literal value

Can you mock secretIdToValue such that get(secretId) returns non-JSON?

dlvenable · 2023-09-15T17:04:17Z

...ugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java

+    private static final String AWS_IAM_ROLE = "role";
+    private static final String AWS_IAM = "iam";
+
+    @JsonProperty("name")


AWS SecretsManager uses the term SecretId, so let's name this secret_id.

https://docs.aws.amazon.com/secretsmanager/latest/apireference/API_GetSecretValue.html#API_GetSecretValue_RequestSyntax

dlvenable · 2023-09-15T17:05:01Z

...ugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java

+                .build();
+    }
+
+    private AwsCredentialsProvider authenticateAwsConfiguration() {


Let's use the existing AWSCredentialsProvider as provided by the AWS Plugin.

dlvenable · 2023-09-15T17:07:40Z

...gins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java

+                    try {
+                        getSecretValueResponse = secretsManagerClient.getSecretValue(getSecretValueRequest);
+                    } catch (Exception e) {
+                        throw ResourceNotFoundException.builder()


Why are you throwing an AWS SDK exception here?

I was trying to wrap it with a more explicit error message. Maybe RuntimeException is more appropriate?

dlvenable · 2023-09-15T17:10:12Z

...rch/src/main/java/org/opensearch/dataprepper/plugins/sink/opensearch/RetryConfiguration.java

@@ -78,7 +78,7 @@ public static RetryConfiguration readRetryConfig(final PluginSetting pluginSetti
    if (dlqFile != null) {
      builder = builder.withDlqFile(dlqFile);
    }
-    final Integer maxRetries = (Integer) pluginSetting.getAttributeFromSettings(MAX_RETRIES);
+    final Integer maxRetries = pluginSetting.getIntegerOrDefault(MAX_RETRIES, null);


Are these part of another PR? This seems unrelated.

Casting would not work for secret value substitution especially when it is a String. But it is possible with generic type in PluginConfigValueTranslator this would not be necessary

Signed-off-by: George Chen <[email protected]>

chenqi0805 added 30 commits May 31, 2023 16:01

ADD: initial implementation on injecting extension config

9b59ec1

Signed-off-by: George Chen <[email protected]>

ENH: context change

43a52d7

Signed-off-by: George Chen <[email protected]>

TST: infra

fb9af26

Signed-off-by: George Chen <[email protected]>

Merge branch 'main' into enh/allow-config-in-extension

f906fce

Signed-off-by: George Chen <[email protected]>

REF: parsing extension from pipeline config

a3b4c24

Signed-off-by: George Chen <[email protected]>

MAINT: remove test code

2049066

Signed-off-by: George Chen <[email protected]>

MAINT: fix test cases

250ae6f

Signed-off-by: George Chen <[email protected]>

ADD: initial boilerplate

b4f73d1

Signed-off-by: George Chen <[email protected]>

TST: test cases

a05352e

Signed-off-by: George Chen <[email protected]>

Merge branch 'main' into enh/allow-config-in-extension

e143fbb

Signed-off-by: George Chen <[email protected]>

MAINT: handle null value on config in the ExtensionPlugin

79f20cf

Signed-off-by: George Chen <[email protected]>

FIX: null validation

50f8e0d

Signed-off-by: George Chen <[email protected]>

Merge branch 'enh/allow-config-in-extension' into add/aws-secret-exte…

c56001b

…nsion Signed-off-by: George Chen <[email protected]>

ADD: config value translater

12e9d32

Signed-off-by: George Chen <[email protected]>

MAINT: test coverage

0f6fbf2

Signed-off-by: George Chen <[email protected]>

REF: pipeline_extensions in data-prepper-config

bd2c0a6

Signed-off-by: George Chen <[email protected]>

Merge branch 'enh/allow-config-in-extension' into add/aws-secret-exte…

1e2bdc9

…nsion Signed-off-by: George Chen <[email protected]>

MAINT: checkpoint

d35cb90

Signed-off-by: George Chen <[email protected]>

MAINT: checkin missing doc

5b98976

Signed-off-by: George Chen <[email protected]>

MAINT: revert changes on refactoring pipeline_extensions

e1dec5b

Signed-off-by: George Chen <[email protected]>

MAINT: revert pipeline_extensions into data-prepper-config

51df8e3

Signed-off-by: George Chen <[email protected]>

REF: pipeline_extensions -> extensions

d545390

Signed-off-by: George Chen <[email protected]>

MAINT: refactor ObjectMapper

bab025d

Signed-off-by: George Chen <[email protected]>

MAINT: unused imports

d7a057d

Signed-off-by: George Chen <[email protected]>

Merge branch 'main' into enh/allow-config-in-extension

1081eb3

Signed-off-by: George Chen <[email protected]>

REF: merge dataflow model

09c2146

Signed-off-by: George Chen <[email protected]>

MAINT: resolve merging conflict

3a84ff3

Signed-off-by: George Chen <[email protected]>

MAINT: import fix

6527e07

Signed-off-by: George Chen <[email protected]>

FIX: do not allow duplicate pipelines

b7c19f3

Signed-off-by: George Chen <[email protected]>

MAINT: revert

7b29f73

Signed-off-by: George Chen <[email protected]>

chenqi0805 added 2 commits September 14, 2023 10:22

MAINT: rootKey -> jsonPath

4f932a3

Signed-off-by: George Chen <[email protected]>

MAINT: workaround for pluginsettings

01e2f94

Signed-off-by: George Chen <[email protected]>

chenqi0805 requested review from engechas, graytaylor0, dinujoh, kkondaka, asifsmohammed, dlvenable and oeyh as code owners September 14, 2023 18:17

MAINT: fix test extesion

fbba530

Signed-off-by: George Chen <[email protected]>

chenqi0805 mentioned this pull request Sep 14, 2023

ENH: support pipeline extensions in pipeline config #3299

Merged

4 tasks

chenqi0805 added 4 commits September 14, 2023 13:49

MAINT: fix ExtensionLoaderTests

ed47582

Signed-off-by: George Chen <[email protected]>

MAINT: inject allowInPipelineConfigurations

472c502

Signed-off-by: George Chen <[email protected]>

MAINT: merge main and resolve conflict

784a8e2

Signed-off-by: George Chen <[email protected]>

MAINT: fix coverage

b47911e

Signed-off-by: George Chen <[email protected]>

dlvenable requested changes Sep 15, 2023

View reviewed changes

chenqi0805 added 6 commits September 15, 2023 13:05

MAINT: unnecessary dependency

8b99927

Signed-off-by: George Chen <[email protected]>

REF: SecretsSupplier package

d065938

Signed-off-by: George Chen <[email protected]>

MAINT: rename name to secret_id

b642ee5

Signed-off-by: George Chen <[email protected]>

MAINT: ResourceNotFoundException -> RuntimeException

af0664b

Signed-off-by: George Chen <[email protected]>

MAINT: remove unused deserializer

7054ca1

Signed-off-by: George Chen <[email protected]>

MAINT: refactor return type

8f90fe0

Signed-off-by: George Chen <[email protected]>

chenqi0805 requested a review from dlvenable September 15, 2023 21:48

chenqi0805 mentioned this pull request Sep 18, 2023

Use AWSPlugin IAM credentials in other extension plugins #3350

Open

MAINT: ref and fix test coverage

73a265d

Signed-off-by: George Chen <[email protected]>

dlvenable approved these changes Sep 18, 2023

View reviewed changes

chenqi0805 merged commit b47b3d5 into main Sep 19, 2023
49 checks passed

chenqi0805 deleted the add/aws-secret-extension branch September 19, 2023 04:48

asifsmohammed pushed a commit to asifsmohammed/data-prepper that referenced this pull request Sep 27, 2023

FEAT: AWS secret extension (opensearch-project#3340)

62e99ef

Signed-off-by: George Chen <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEAT: AWS secret extension #3340

FEAT: AWS secret extension #3340

chenqi0805 commented Sep 14, 2023 •

edited

Loading

github-actions bot commented Sep 14, 2023 •

edited

Loading

dlvenable Sep 15, 2023

dlvenable Sep 15, 2023

dlvenable Sep 15, 2023

chenqi0805 Sep 15, 2023

dlvenable Sep 15, 2023

chenqi0805 Sep 15, 2023

dlvenable Sep 18, 2023

dlvenable Sep 15, 2023

dlvenable Sep 15, 2023

chenqi0805 Sep 18, 2023

dlvenable Sep 15, 2023

chenqi0805 Sep 15, 2023

dlvenable Sep 15, 2023

chenqi0805 Sep 15, 2023

		@@ -0,0 +1,7 @@
		package org.opensearch.dataprepper.aws.api;

		public interface SecretsSupplier {

FEAT: AWS secret extension #3340

FEAT: AWS secret extension #3340

Conversation

chenqi0805 commented Sep 14, 2023 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Sep 14, 2023 • edited Loading

Unit Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenqi0805 commented Sep 14, 2023 •

edited

Loading

github-actions bot commented Sep 14, 2023 •

edited

Loading